Skip to content

feat(sdk): automate MinSdkVersion.VALUE maintenance via release pipeline (#36698) - #522

Open
KevinDavilaDotCMS wants to merge 1 commit into
mainfrom
36698-clean-attempt
Open

feat(sdk): automate MinSdkVersion.VALUE maintenance via release pipeline (#36698)#522
KevinDavilaDotCMS wants to merge 1 commit into
mainfrom
36698-clean-attempt

Conversation

@KevinDavilaDotCMS

Copy link
Copy Markdown
Member

Test implementation for dotCMS#36698 — testing in this fork before porting to the real repo. Supersedes #521 (rebased cleanly onto origin/main).

…ine (dotCMS#36698)

- New bump_min_sdk_version input on cicd_6-release.yml
- New verify-branch validation: fails the release if a merged PR carries
  an SDK-breaking-change label but the input was left false (checks for
  a stale unmerged bump PR too, non-fatally)
- New bump-min-sdk-version job, gated on release-prepare+build+deployment
  succeeding, that opens a PR against main (never a direct push) bumping
  MinSdkVersion.VALUE and notifies Slack for human review. Idempotent:
  no-ops if main already has the value, dedupes an already-open PR.
- New ai_claude-sdk-breaking-change.yml workflow mirroring
  ai_claude-rollback-safety.yml, labeling PRs AI: SDK Breaking Change /
  AI: Not SDK Breaking Change against a new reference doc
- New docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md grounded in actual
  @dotcms/client code (GraphQL page API, REST nav/content responses,
  UVE postMessage protocol, the compatibility headers themselves)
- Updated MinSdkVersion.java Javadoc to describe the new automated flow
- Also carries the SDK compatibility handshake sync (MinSdkVersion.java,
  SdkVersionWebInterceptor.java, sdk-compatibility.ts, etc.) and the
  already-verified next-tag publish fixes (continue-on-error +
  empty-version fallback), both needed as a baseline for this work.
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

Claude finished @KevinDavilaDotCMS's task in 1m 22s —— View job


Rollback-Safety Analysis

  • Read docs/core/ROLLBACK_UNSAFE_CATEGORIES.md
  • Get full PR diff (2f4ab816...b5c52931)
  • Analyze diff against every unsafe category
  • Post findings / apply label

Result: ✅ Safe to rollback — no unsafe category matched.

  • MinSdkVersion.java / SdkVersionWebInterceptor.java only add two new response headers (X-DotCMS-Version, X-DotCMS-Min-SDK); no schema, ES mapping, or storage change.
  • InterceptorFilter.java only registers the new interceptor into the chain — no existing behavior removed or altered.
  • SDK/frontend changes (sdk-compatibility.ts, fetch-http-client.ts, rollup/jest config) are additive and fail-open on the client side; they don't change any REST/GraphQL response contract.
  • CI/CD workflow changes (cicd_3-trunk.yml, cicd_6-release.yml, new ai_claude-sdk-breaking-change.yml) are pipeline/release automation only — no runtime data-layer effect.
  • No runonce DB tasks, DROP/RENAME operations, PK restructuring, or ES mapping changes anywhere in the diff.

Label AI: Safe To Rollback applied to this PR.

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 dotBot Review (Bedrock)

Reviewed 15 file(s); 11 candidate(s) → 9 confirmed, 0 uncertain (unverified, kept for review).

Confirmed findings

  • 🟠 High .github/workflows/cicd_3-trunk.yml:195 — Missing error message propagation from next-version step to Slack notifications
    The next-version step lacks error output capture. Slack notification uses generic needs.build.result without step-specific error details, forcing log inspection for diagnosis. No evidence of error message being set as output in failing steps.
  • 🟡 Medium .github/workflows/cicd_6-release.yml:148 — PR number extraction regex misses multiple PR references per commit line
    The regex .*\(#[0-9]\+\).* captures only the last PR number in each commit line due to greedy .* matches. Commit messages containing multiple PR references (e.g. 'Fix Issue 29180 commit hook does not work in windows wsl ubuntu #123 and chore(deps): update redis docker tag to v6.2.12 #456') will only capture chore(deps): update redis docker tag to v6.2.12 #456, missing earlier references.
  • 🟡 Medium .github/workflows/cicd_6-release.yml:345 — Sed pattern for MinSdkVersion.VALUE replacement assumes specific formatting
    The sed command uses a regex that matches 'MinSdkVersion.VALUE = .*' which assumes no trailing comments or line breaks. If the source line contains comments (e.g., 'MinSdkVersion.VALUE = 24; // Default version') or is split across lines, the replacement would either preserve the comment (leaving stale data) or fail entirely. This could lead to incorrect Min SDK version being set in releases.
  • 🟡 Medium core-web/libs/sdk/client/rollup.config.cjs:44 — Missing error handling for package.json read/parse
    The code uses fs.readFileSync and JSON.parse without try-catch blocks. If package.json is missing or invalid, this will throw unhandled exceptions with raw Node.js stack traces instead of actionable build errors.
  • 🟡 Medium core-web/libs/sdk/client/src/lib/client/adapters/fetch-http-client.spec.ts:40 — Hardcoded SDK version in test
    The test uses a hardcoded '0.0.0-test' version instead of dynamically retrieving the actual SDK version. This creates a false positive scenario where compatibility checks pass in tests but fail in production if the real version isn't properly injected. Tests should validate against the dynamically set SDK version to ensure versioning integrity matches release processes.
  • 🟡 Medium core-web/libs/sdk/client/src/lib/client/adapters/fetch-http-client.ts:76 — Unhandled promise rejection risk in SDK compatibility check
    The checkSdkCompatibility call at line 76 lacks error handling for potential async operations. In the context of SDK version automation (PR Automate MIN_SDK_VERSION bump via release pipeline + AI-based SDK-breaking-change detection core#36698), this function likely performs version validation that may involve Promise-based operations. Without await or .catch(), any rejection would cause unhandled promise errors in both Node.js and browser environments.
  • 🟡 Medium core-web/libs/sdk/client/src/lib/utils/sdk-compatibility.ts:87 — Unparseable server versions default to 0.0.0
    The code uses null coalescing (??) to treat unparseable versions as 0.0.0, which would incorrectly compare snapshot/alpha versions as ancient releases and suppress warnings about malformed version strings that should be investigated. This hides potential compatibility issues and version format errors from users.
  • 🟡 Medium dotCMS/src/main/java/com/dotcms/filters/interceptor/meta/SdkVersionWebInterceptor.java:49 — Missing test coverage for exception handling paths
    The SdkVersionWebInterceptorTest.java file does not exist, indicating no test coverage for the interceptor's error handling (e.g., exceptions during header injection). The interceptor contains a try-catch block (line 49) that would log errors, but without tests, these paths remain unverified.
  • 🟡 Medium dotCMS/src/main/java/com/dotmarketing/filters/InterceptorFilter.java:58 — New SdkVersionWebInterceptor added without test coverage
    The SdkVersionWebInterceptor is added to the interceptor chain in InterceptorFilter.java line 58, but no corresponding test exists in InterceptorFilterTest.java. Grep confirms no test files reference the new interceptor, and InterceptorFilterTest.java (lines 1-100 shown) contains no tests for this component. This leaves the new SDK version header logic untested.

us.deepseek.r1-v1:0 · Run: #30144996867 · tokens: in: 72059 · out: 29211 · total: 101270 · calls: 37 · est. ~$0.255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants